fix(daemon): reset turnCompletedCleanly on same-run retry teardown by tomsen02 · Pull Request #5341 · nexu-io/open-design

tomsen02 · 2026-07-09T07:50:49Z

Why

I'm hardening the daemon's run-lifecycle / event-retention path and hit this while auditing the same-run retry teardown. run.turnCompletedCleanly is a run-scoped success override, but the teardown that recycles the run object between attempts doesn't clear it — so a clean-but-empty attempt 1 silently vouches for a crashed attempt 2. The pain: a genuinely failed run is reported succeeded (blank result shown as success), and run_retry_finished reports retry_result: success, which also skews retry telemetry.

This is the same flag as #4197 (set too early) and #5159 (set when it shouldn't be), but a distinct root cause — the flag being carried forward across a retry. Neither of those touches tearDownAttemptForRetry.

What users will see

A run whose retried attempt crashes with no output now correctly shows as failed instead of a blank "success". No new surface; behavior correction only.

Surface area

None — internal daemon fix + regression test only.

Bug fix verification

Test path that reproduces the bug: apps/daemon/tests/retry-stale-turn-completed-flag.test.ts — drives the full run lifecycle over the daemon HTTP API with a fake claude CLI (clean-but-empty turn → hang → watchdog retry → crash).
Did the test go red on main and green on this branch? yes — deterministic 8/8 red on main, 8/8 green with the fix (the watchdog window is set wide enough that attempt 1's turn_end is always processed first, so there's no flaky false-green).
Additional live verification (real daemon, production HTTP API, same fake CLI): 6/6 succeeded before the fix, 5/5 failed after — the one-line teardown reset is the only variable that flips it.

Validation

pnpm guard (78 pass) + pnpm --filter @open-design/daemon exec tsc -p tsconfig.json --noEmit (clean)
pnpm --filter @open-design/daemon test for the touched area: retry-stale-turn-completed-flag, run-retry-policy, run-retry-runtime, run-failure-classification, run-lifecycle-tracer, daemon-lifecycle, chat-run-artifact-quiet-period, headless-runs — all green (133+ tests)

A same-run retry reuses one run object across attempts. tearDownAttemptForRetry cleared the per-attempt fields (child, exitCode, signal, error, stdinOpen) before re-spawning but left run.turnCompletedCleanly untouched. That flag is set by a clean `turn_end` (applyClaudeStreamJsonRunBookkeeping) and read by the terminal classifier: when attempt 1 finished a clean-but-empty turn (retryable no-output timeout) and attempt 2 then crashed (exit 1, no output), the crashed run was classified 'succeeded' off attempt 1's stale flag — and reported retry_result: success in telemetry. Clear the flag in teardown alongside the other per-attempt resets, with an invariant comment so future per-attempt state that feeds terminal classification is cleared in the same place. Regression: tests/retry-stale-turn-completed-flag.test.ts drives the full run lifecycle over the daemon HTTP API with a fake claude CLI (empty turn then hang, crash on retry). Red on main, green with this fix; deterministic 8/8 both ways.

lefarcen · 2026-07-09T07:54:00Z

🧪 Queued for QA validation — this PR touches a run-lifecycle path that needs a manual QA pass before merge. Nothing needed from you right now; we'll update here once it’s validated. Thanks for the thorough repro + verification notes.

nettee

@tomsen-ai I reviewed the retry teardown change in apps/daemon/src/server.ts and the new end-to-end regression in apps/daemon/tests/retry-stale-turn-completed-flag.test.ts. The reset in tearDownAttemptForRetry() matches the close-classification path that still reads run.turnCompletedCleanly, and the new test exercises the exact stale-flag sequence that was misclassifying a crashed retry as success. I couldn’t rerun the daemon Vitest target in this reviewer worktree because dependencies are not installed here (vitest/node_modules missing), but the code path and test coverage both look coherent. Nice catch and a tight regression harness.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

AmyShang-alt · 2026-07-10T03:57:30Z

QA validation passed for head a0f2896ab223b68c9b71c2f77cbad1cdfe4973bf.

Local targeted validation:

PASS pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/retry-stale-turn-completed-flag.test.ts — 1 test passed; covers the stale turnCompletedCleanly same-run retry regression.
PASS targeted adjacent daemon suite: retry-stale-turn-completed-flag, run-retry-policy, run-failure-classification, run-lifecycle-tracer, daemon-lifecycle, chat-run-artifact-quiet-period, headless-runs — 7 files passed, 130 tests passed.
PASS pnpm --filter @open-design/daemon exec tsc -p tsconfig.json --noEmit.

Remote PR checks were refreshed and the relevant CI jobs are green. I also checked tests/run-retry-runtime.test.ts; it currently fails with scanRunEventsForUsageAnalytics is not defined, but the same failure reproduces on latest origin/main, so I am treating that as a pre-existing baseline issue rather than a regression from this PR.

Marking this runtime validation as passed.

AmyShang-alt

QA validation passed on head a0f2896; approving.

open-design-bot · 2026-07-10T04:05:05Z

🎉 📡 You just leveled up to Giotto

📡 ✨ Sending steady signals.

🙌 Your contributions are sending a clear signal across the network: you care about making Open Design better. Keep transmitting.

💛 Thanks for helping Open Design move forward. Keep building in the open. 🚀

📊 Rank #56 among 400+ contributors

🔗 Share on X (English) · 分享到 X（中文）

lefarcen mentioned this pull request Jul 9, 2026

[Bug]: same-run retry inherits attempt 1's clean-turn flag, classifying a crashed retry as succeeded #5340

Closed

lefarcen requested a review from nettee July 9, 2026 07:54

lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 9, 2026

nettee approved these changes Jul 9, 2026

View reviewed changes

lefarcen requested a review from AmyShang-alt July 9, 2026 08:06

AmyShang-alt added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 10, 2026

AmyShang-alt approved these changes Jul 10, 2026

View reviewed changes

AmyShang-alt added this pull request to the merge queue Jul 10, 2026

Merged via the queue into nexu-io:main with commit 91179e6 Jul 10, 2026
31 checks passed

This was referenced Jul 12, 2026

[Bug]: a Task sub-agent's end_turn is treated as the main turn's completion — a crashed run is misclassified as succeeded #5487

Closed

fix(daemon): do not treat a Task sub-agent turn_end as the main turn's completion #5488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(daemon): reset turnCompletedCleanly on same-run retry teardown - #5341

fix(daemon): reset turnCompletedCleanly on same-run retry teardown#5341
AmyShang-alt merged 1 commit into
nexu-io:mainfrom
tomsen02:fix/retry-stale-turn-completed-flag

tomsen02 commented Jul 9, 2026

Uh oh!

lefarcen commented Jul 9, 2026

Uh oh!

nettee left a comment

Uh oh!

AmyShang-alt commented Jul 10, 2026

Uh oh!

AmyShang-alt left a comment

Uh oh!

Uh oh!

open-design-bot Bot commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

tomsen02 commented Jul 9, 2026

Why

What users will see

Surface area

Bug fix verification

Validation

Uh oh!

lefarcen commented Jul 9, 2026

Uh oh!

nettee left a comment

Choose a reason for hiding this comment

Uh oh!

AmyShang-alt commented Jul 10, 2026

Uh oh!

AmyShang-alt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

open-design-bot Bot commented Jul 10, 2026

🎉 📡 You just leveled up to Giotto

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants